Skip to content

Conversation

widlarizer
Copy link
Collaborator

I believe driving the bit of a signal with a constant x-bit is exactly the same as not driving it at all. This PR removes this inconsistency by extending SigPool. As the SigPool is used to check the assign_mapped wire bits against unmapped bits, this creates an arbitrary distinction between driven and undriven. I have added a pass-specific mechanism by inheriting from SigPool that happens to work for this case. I don't know if SigPool is similarly misused elsewhere.

This PR shares its motivation with #5003

@widlarizer widlarizer marked this pull request as draft April 10, 2025 09:39
@widlarizer
Copy link
Collaborator Author

This doesn't create the ideal behavior and the implementation is hard to defend either. Looking into alternatives

@widlarizer widlarizer force-pushed the emil/fix-x-check_opt branch from 42c0f9f to 532f9ab Compare April 23, 2025 20:41
@widlarizer widlarizer marked this pull request as ready for review April 24, 2025 13:22
@KrystalDelusion
Copy link
Member

Might be good to add some tests, in particular for checking that opt_clean -x and setundef -undriven -undef; opt_clean are equivalent.
Am I also right in saying that opt -full wouldn't benefit from this, since opt_expr -full (or more specifically, opt_expr -undriven) already replaces undriven bits with x-bits?

@KrystalDelusion
Copy link
Member

KrystalDelusion commented Apr 24, 2025

Also given that setundef and opt_expr both use -undriven and refer to it as "replace undriven nets with undef (x) constants" in the help, it would be good to be consistent here (unless I'm misinterpreting, and this isn't the same).

@jix jix self-assigned this Aug 11, 2025
@jix
Copy link
Member

jix commented Oct 9, 2025

I think in a synthesis context it makes more sense to treat constant 'z as undriven and 'x as driven but don't care, but also (optionally?) allow used-but-never-driven values to be converted into don't cares (producing a warning when that happens). Without that distinction, I'd be worried about what could happen to bidirectional signals.

For bufnorm I also had to add code to opt_clean to handle a buffered 'z signal as undriven, which moves us slightly in that direction. It doesn't really make sense from an EE perspective, but some way to mark a subset of the outputs bits of a $buf as undriven was an absolute requirement to make bufnorm work in the presence of bidirectional connections, and that seemed like the least risky choice.

While that was a somewhat forced step, I feel like we should have a discourse thread on how to make 'z, 'x and undriven more consistent and meaningful before deciding to merge changes to specific passes. Especially if there is a chance we will replace bufnorm with signorm which would allow us to undo this forced step.

@jix jix added the discuss needs further discussion on the YosysHQ discourse (https://yosyshq.discourse.group) label Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

discuss needs further discussion on the YosysHQ discourse (https://yosyshq.discourse.group)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants